Skip to content

Use longer server start timeout by default#40

Merged
mlackman merged 2 commits intoUpCloudLtd:masterfrom
scop:start-timeout
Jan 31, 2018
Merged

Use longer server start timeout by default#40
mlackman merged 2 commits intoUpCloudLtd:masterfrom
scop:start-timeout

Conversation

@scop
Copy link
Copy Markdown
Member

@scop scop commented Jan 30, 2018

No description provided.

scop added 2 commits January 30, 2018 12:50
Leave as default (-1) to fall back to timeout set in base api
instance. Set to None to disable timing out altogether.
The 10 second CloudManager default is not quite sufficient here.
self.timeout = timeout

def request(self, method, endpoint, body=None):
def request(self, method, endpoint, body=None, timeout=-1):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer timeout to be None

Copy link
Copy Markdown
Member Author

@scop scop Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point for the default not to be None is that one can then call the method with timeout=None to disable the timeout altogether. (See complete commit message of the first commit.) That's consistent with how one can disable timeouts for the whole manager.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, works for me :D, merging

else:
json_body_or_None = None

call_timeout = timeout if timeout != -1 else self.timeout
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When timeout is None, this could be stated as: call_timeout = timeout or self.timeout

@mlackman mlackman merged commit 119b8df into UpCloudLtd:master Jan 31, 2018
@scop scop deleted the start-timeout branch January 31, 2018 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants